Skip to content

[PM-35253] Add organization ability UseInviteLinks#7489

Merged
r-tome merged 21 commits intomainfrom
ac/pm-35253/add-useinvitelinks-organization-ability
Apr 30, 2026
Merged

[PM-35253] Add organization ability UseInviteLinks#7489
r-tome merged 21 commits intomainfrom
ac/pm-35253/add-useinvitelinks-organization-ability

Conversation

@r-tome
Copy link
Copy Markdown
Contributor

@r-tome r-tome commented Apr 17, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-35253

📔 Objective

Add the new organization ability UseInviteLinks across the server (schema/migrations, domain + licensing, API models, and tests).

A database data migration is included to enable UseInviteLinks for existing Enterprise organizations.

Related Pricing Service changes: https://github.com/bitwarden/billing-pricing/pull/103
Related Clients changes: bitwarden/clients#20227

@r-tome r-tome added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 17, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds the UseInviteLinks organization ability end-to-end across the server: MSSQL schema (table, stored procedures, views), EF migrations for MySQL/Postgres/SQLite, the Organization entity, licensing claims and verification, API response models, plan features, and the OrganizationAbility projection. It includes a batched MSSQL data migration (with EF equivalents) to enable the feature for existing Enterprise organizations across all historical plan types (4, 5, 10, 11, 14, 15, 19, 20), and adds integration tests validating the migration applies correctly to Enterprise plans and not to non-Enterprise plans, with idempotency coverage.

Code Review Details

No actionable findings. The implementation follows the established pattern for similar recently-added abilities (UseMyItems, UsePhishingBlocker, UseDisableSmAdsForUsers):

  • License hash exclusion is correctly added in OrganizationLicense.GetDataBytes alongside the other post-v15 claims, preserving backward compatibility with older license files.
  • VerifyData uses the HasClaim guard pattern so older licenses without the UseInviteLinks claim still validate.
  • The SQL data migration is batched (TOP (@BatchSize) with a self-converging UseInviteLinks = 0 predicate) and the reviewer has already validated runtime (~1 minute against backup).
  • Column ordering in Organization.sql was addressed in the prior review thread (now placed after ExemptFromBillingAutomation).
  • PlanType integer literals in the data migrations match the PlanType enum for all Enterprise variants.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026

Logo
Checkmarx One – Scan Summary & Detailsc1e4e971-f54f-400c-b5fd-18ebbb065304


New Issues (9) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 MEDIUM CSRF src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 393
detailsMethod at line 393 of /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs gets a parameter from a user request from model. This par...
Attack Vector
2 MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 151
detailsMethod at line 151 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from model. This parameter valu...
Attack Vector
3 MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 127
detailsMethod at line 127 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from model. This parameter valu...
Attack Vector
4 MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/MembersController.cs: 232
detailsMethod at line 232 of /src/Api/AdminConsole/Public/Controllers/MembersController.cs gets a parameter from a user request from model. This parame...
Attack Vector
5 MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 275
detailsMethod at line 275 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from orgUserId. This parameter ...
Attack Vector
6 MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/MembersController.cs: 269
detailsMethod at line 269 of /src/Api/AdminConsole/Public/Controllers/MembersController.cs gets a parameter from a user request from model. This parame...
Attack Vector
7 MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 167
detailsMethod at line 167 of /src/Api/AdminConsole/Public/Controllers/GroupsController.cs gets a parameter from a user request from model. This paramet...
Attack Vector
8 MEDIUM CSRF src/Api/Vault/Controllers/CiphersController.cs: 1558
detailsMethod at line 1558 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
9 MEDIUM CSRF src/Api/Vault/Controllers/CiphersController.cs: 1385
detailsMethod at line 1385 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector

Fixed Issues (7) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 275
MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 127
MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 151
MEDIUM CSRF src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 393
MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/MembersController.cs: 232
MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/MembersController.cs: 269
MEDIUM CSRF src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 164

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 97.29730% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.88%. Comparing base (27ae3d5) to head (a8a8c26).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../Core/AdminConsole/Services/OrganizationFactory.cs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7489      +/-   ##
==========================================
+ Coverage   63.87%   63.88%   +0.01%     
==========================================
  Files        2088     2088              
  Lines       92350    92383      +33     
  Branches     8205     8205              
==========================================
+ Hits        58987    59019      +32     
- Misses      31337    31338       +1     
  Partials     2026     2026              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@r-tome r-tome marked this pull request as ready for review April 17, 2026 13:34
@r-tome r-tome requested review from a team as code owners April 17, 2026 13:34
Comment thread src/Sql/dbo/Tables/Organization.sql Outdated
[UsePhishingBlocker] BIT NOT NULL CONSTRAINT [DF_Organization_UsePhishingBlocker] DEFAULT (0),
[UseDisableSmAdsForUsers] BIT NOT NULL CONSTRAINT [DF_Organization_UseDisableSmAdsForUsers] DEFAULT (0),
[UseMyItems] BIT NOT NULL CONSTRAINT [DF_Organization_UseMyItems] DEFAULT (0),
[UseInviteLinks] BIT NOT NULL CONSTRAINT [DF_Organization_UseInviteLinks] DEFAULT (0),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ PR 7438 has /util/Migrator/DbScripts/2026-04-10_02_AddExemptFromBillingAutomation.sql, which adds the ExemptFromBillingAutomation column to this table. If this PR is deployed with the same release (or later) as PR 7438, the columns will be out of order. Do you know if this deployment will happen before 7438? If not, then the UseInviteLinks column should be placed after the ExemptFromBillingAutomation, and the other objects should be adjusted as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkincaid-bw well observed! I have fixed the order of the columns by putting UseInviteLinks after ExemptFromBillingAutomation

JimmyVo16
JimmyVo16 previously approved these changes Apr 17, 2026
mkincaid-bw
mkincaid-bw previously approved these changes Apr 21, 2026
Copy link
Copy Markdown
Contributor

@mkincaid-bw mkincaid-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


WHILE @RowsAffected > 0
BEGIN
UPDATE TOP (@BatchSize) [dbo].[Organization]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I tested this against a backup and it took about a minute to run.

@r-tome r-tome removed ai-review-vnext Request a Claude code review using the vNext workflow needs-qa labels Apr 28, 2026
@sonarqubecloud
Copy link
Copy Markdown

@r-tome r-tome merged commit 52d9a9c into main Apr 30, 2026
47 of 48 checks passed
@r-tome r-tome deleted the ac/pm-35253/add-useinvitelinks-organization-ability branch April 30, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants